Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #17 +/- ##
==========================================
+ Coverage 73.38% 74.28% +0.89%
==========================================
Files 5 5
Lines 620 591 -29
==========================================
- Hits 455 439 -16
+ Misses 165 152 -13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| A2 = copy(A) | ||
| Q, R = qr!(A2) | ||
| Ri = ldiv!(UpperTriangular(R)', TensorKit.MatrixAlgebra.one!(similar(R))) | ||
| Q, R = LinearAlgebra.qr!(A2) |
There was a problem hiding this comment.
Why not use MAK.qr_compact! for this?
There was a problem hiding this comment.
same for the other qr! calls below.
There was a problem hiding this comment.
Because this is the compact form that is immediately in-place multiplied into a different matrix below, so technically that's doing an additional multiplication lmul! below. I'm not sure if it really matters, but it felt a bit weird to change this into something slower if it was working before :)
|
On this evening's flight, I will try to spend some time on TensorKitManifolds.jl, to indeed properly migrate some functionality to MatrixAlgebraKit and update it to TensorKit 0.15. Let's see where I land (hopefully Krakow, but you get the point). |
|
@Jutho, do you mind if I tag and release this as a patch version already? I think this counts as non-breaking, and it is holding back MPSKit from using the new TensorKit version (and therefore also PEPSKit and TNRKit). Definitely happy to improve to more MatrixAlgebraKit implementations afterwards. |
|
Ok sure. |
2 participants
This is a minimal set of changes needed to make this package compatible with TK v0.15.
In the long run, it might be nice to consider making some more substantial changes to further reduce the required code here, while migrating some of it to MatrixAlgebraKit.
I'm listing some of these suggestions here as a reminder:
PolarNewtonis an algorithm for computing the polar decomposition, which we can directly implement in MatrixAlgebraKit._left_polar!could be replaced withMatrixAlgebraKit.left_polar!if we adopt a strategy that is similar to the one we have for QR decompositions -- providing an emptyRto indicate that we do not wish to computeR.project(anti)hermitianfunctions might also be reasonable inMatrixAlgebraKit.For now though, I'd prefer to just leave that for the future and get this merged, so also MPSKit can be updated.
Before tagging a new release, we could consider switching the formatter, and optionally migrating the repository to use the shared github actions.